Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better masking of AWS secret access keys in HTTP request/response logs #560

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Better masking of AWS secret access keys in HTTP request/response logs #560

merged 3 commits into from
Jul 31, 2023

Conversation

ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit commented Jul 21, 2023 •
edited
Loading

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Adds MaskAWSSecretKeys, implementing the logic described here to detect AWS secret access keys: Find me 40-character, base-64 strings that don’t have any base 64 characters immediately before or after.

Closes #559.

% go test ./logging
ok  	github.com/hashicorp/aws-sdk-go-base/v2/logging	0.441s

@ewbankkit ewbankkit requested a review from a team as a code owner July 21, 2023 18:39
jar-b
jar-b approved these changes Jul 21, 2023
View reviewed changes
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

$ go test -v ./logging/... | rg "PASS|FAIL"
--- PASS: TestMaskAWSSensitiveValues (0.00s)
    --- PASS: TestMaskAWSSensitiveValues/mask_simple (0.00s)
    --- PASS: TestMaskAWSSensitiveValues/mask_xml (0.00s)
    --- PASS: TestMaskAWSSensitiveValues/no_mask (0.00s)
    --- PASS: TestMaskAWSSensitiveValues/mask_multiple_json (0.00s)
    --- PASS: TestMaskAWSSensitiveValues/mask_complex_json (0.00s)
PASS

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Jul 24, 2023 •
edited
Loading

TODO

Mask AWS_SESSION_TOKEN value.

From here:

The size of the security token that AWS STS API operations return is not fixed. We strongly recommend that you make no assumptions about the maximum size.

gdavison
gdavison approved these changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🚀

@gdavison gdavison merged commit 87bbc87 into main Jul 31, 2023
6 checks passed
@ewbankkit ewbankkit deleted the sensitive-value-masking branch August 1, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdavison gdavison gdavison approved these changes

@jar-b jar-b jar-b approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Overzealous sensitive value masking in HTTP request/response logs
3 participants
@ewbankkit @gdavison @jar-b